Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(hesai): software-based precise FoV limit & early publishing #173

Merged
merged 73 commits into from
Sep 18, 2024

Conversation

mojomex
Copy link
Collaborator

@mojomex mojomex commented Jun 27, 2024

PR Type

  • New Feature

Description

This PR introduces software-based FoV limits and early publishing for Hesai sensors.

Software-based FoV

Previously, configuring cloud_min_angle and cloud_max_angle would set the sensor's hardware filter to those values.
However, the hardware filter is using raw azimuths.
This lead to the scan having jagged edges (points missing in some channels, too many points in others).
For certain models, this affected areas of up to ~10deg on both edges of the FoV.

This PR improves upon this in the following way:

  1. User sets cloud_min_angle and cloud_max_angle to the exact FoV they want the output pointcloud to have
  2. Nebula retrieves sensor calibration file and computes appropriately over-sized* hardware-FoV
  3. Nebula sends hardware-FoV to sensor (or validates it if setup_sensor == false)
  4. The Nebula decoder cuts the scan at the exact edges defined via the parameters above

Early publishing

Previously, a scan was published once the packet after it was received. This was done because this made the scan completion check easier (could compare 2 azimuths according to some scan completion criterion).
For scans of 360deg, this is not a problem, as there is no time gap between the end of one scan and the beginning of the next.
However, for FoVs < 360deg, this approach increases latency unnecessarily: for an FoV of 180deg and 1 full rotation every 100ms, the additional latency of the finished pointcloud just "sitting there" is 50ms.

This PR improves upon this in the following way:

  1. Swap the order of decoding and scan completion check: decode first, then check for completion
  2. Check if the current packet completed the current scan, as opposed to whether the current packet is the first belonging to the next scan

Other

For testing this PR with AT128, I had to fix the LidarRange issue:

This was done by checking the sensor model before sending the LidarRange command.

Review Procedure

Check for the following FoVs

  • 0 - 360 deg
  • 90 - 270 deg
  • 270 - 90 deg
    if the pointcloud in Rviz has clean edges at those angles for a 360 deg sensor such as OT128.

Check AT128 in the same way but with FoVs inside [30; 150].

Remarks

These two improvements have been combined into one PR as they are closely related and would require extra work to function independently.

Pre-Review Checklist for the PR Author

PR Author should check the checkboxes below when creating the PR.

  • Assign PR to reviewer

Checklist for the PR Reviewer

Reviewers should check the checkboxes below before approval.

  • Commits are properly organized and messages are according to the guideline
  • (Optional) Unit tests have been written for new behavior
  • PR title describes the changes

Post-Review Checklist for the PR Author

PR Author should check the checkboxes below before merging.

  • All open points are addressed and tracked via issues or tickets

CI Checks

  • Build and test for PR: Required to pass before the merge.

@mojomex
Copy link
Collaborator Author

mojomex commented Jul 2, 2024

🚧 Evaluation

Pointclouds are colored by relative point timestamp: red is 0 ms relative to the cloud timestamp, blue is 100 ms.

🟢 360deg rotational sensors

FoV Setting (min, max, cut) Resulting pointcloud
0, 360, 0 0-360-0
0, 360, 180 0-360-180
0, 360, 360 Cut angle gets overridden with 0, result is the same as 0, 360, 0: Cut angle was set to 360 deg, overriding with canonical representation of 0 deg.
90, 270, 90 90-270-90
90, 270, 180 90-270-180
90, 270, 270 90-270-270
270, 90, 90 270-90-90
270, 90, 0 270-90-0
270, 90, 270 270-90-270

🟢 AT128 (120deg rotational sensor)

FoV Setting (min, max, cut) Resulting pointcloud
30, 150, 30 Nebula rejects the parameter: Cannot cut scan right at the start of the FoV. Cut at the end instead.
30, 150, 31 30-150-31
30, 150, 90 30-150-90
30, 150, 149 30-150-149
30, 150, 150 30-150-150
45, 135, 45 Nebula rejects the parameter: Cannot cut scan right at the start of the FoV. Cut at the end instead.
45, 135, 46 45-135-46
45, 135, 90 45-135-90
45, 135, 134 45-135-134
45, 135, 135 45-135-135

@mojomex mojomex changed the title feat(hesai): software-based precise FoV limit feat(hesai): software-based precise FoV limit & early publishing Jul 2, 2024
@mojomex mojomex requested a review from drwnz July 2, 2024 12:55
@mojomex mojomex marked this pull request as ready for review July 2, 2024 12:56
@mojomex mojomex force-pushed the hesai-better-fov branch 2 times, most recently from 6e61e50 to 9e72d84 Compare July 9, 2024 07:45
@mojomex mojomex marked this pull request as draft July 11, 2024 07:08
Copy link
Collaborator

@drwnz drwnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was tested with 40P with the following issue:

  • Setting cut angle out side of the range of FoV start and end, resulted in no error and also pointcloud output.
  • Even if that was the case and the "cut" happens in the area outside of the FoV (which is kind of uneccesary but logically possible) and the actual observed point cloud is correct in terms of min and max angles.
  • Example: cut 10, min 180, max 360: no warnings, output pointcloud is full 180 degree segment.

@mojomex
Copy link
Collaborator Author

mojomex commented Sep 17, 2024

@drwnz Thank you for the review!

I have addressed your comments and re-tested setting the cut_angle parameter to a value outside of the FoV.
Nebula now properly emits an error:

[INFO] [hesai_ros_wrapper_node]: OnParameterChange
[ERROR] [hesai_ros_wrapper_node]: Cannot cut scan outside of the FoV.
[WARN] [hesai_ros_wrapper_node]: OnParameterChange aborted: Could not set SensorConfiguration

The fix is in this commit: 2e42bd0

@mojomex mojomex requested a review from drwnz September 17, 2024 06:12
Copy link
Collaborator

@drwnz drwnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a few more comments, nothing urgent.
I haven't gone through the tests except for verifying that they passed.

@mojomex mojomex requested a review from drwnz September 18, 2024 07:29
Copy link
Collaborator

@drwnz drwnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ready to go, great work!

@mojomex mojomex merged commit 16e1489 into tier4:develop Sep 18, 2024
10 checks passed
@mojomex mojomex deleted the hesai-better-fov branch September 18, 2024 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants